Settings file creation and validation#2176
Settings file creation and validation#2176liamjpeters wants to merge 6 commits intoPowerShell:mainfrom
Conversation
…onfigurable rules
- Implemented `New-ScriptAnalyzerSettingsFile` cmdlet to create a new PSScriptAnalyzer settings file, with options for presets and overwriting existing files. - Added `Test-ScriptAnalyzerSettingsFile` cmdlet to validate settings files, checking for parseability, rule existence, and valid options. - Created comprehensive tests for both cmdlets to ensure functionality and error handling. - Updated module manifest to export the new cmdlets. - Added documentation for both cmdlets, including usage examples and parameter descriptions. - Enhanced error messages in the strings resource file for better clarity during validation failures.
- Update Helper.cs to return null for empty output paths instead of an empty array. - Add new error message for invalid option types in Strings.resx. - Extend tests for New-ScriptAnalyzerSettingsFile to check for new keys: CustomRulePath, IncludeDefaultRules, and RecurseCustomRulePath. - Modify Test-ScriptAnalyzerSettingsFile tests to validate output and error handling for various scenarios, including type mismatches and invalid values. - Improve documentation for New-ScriptAnalyzerSettingsFile and Test-ScriptAnalyzerSettingsFile to clarify behavior and parameters, including handling of custom rules and output format.
andyleejordan
left a comment
There was a problem hiding this comment.
Thanks Liam — I reviewed this with help from Claude Opus 4.7: Code quality is high (clean separation of formatting/validation/AST helpers, defensive null checks, proper ShouldProcess on New-ScriptAnalyzerSettingsFile), the new public surface is purely additive (new cmdlets, a new Options property on RuleInfo via a new constructor overload — existing constructors and consumers untouched), and test coverage is genuinely thorough on both happy and error paths.
A few thoughts before I approve:
- Maintenance cost is real but bounded. Once these ship they're public API; future rule additions will need their configurable options to round-trip cleanly through
RuleOptionInfo, andTest-ScriptAnalyzerSettingsFilebecomes coupled to the.psd1settings schema. The enum-discovery heuristic inRuleOptionInfo(the naming-convention match on the option's property type) is clever but silent on mismatch — worth either documenting the convention or considering an explicit attribute, so a future contributor doesn't accidentally ship a rule whosePossibleValuescome out empty. - Relationship to #2134. You've explicitly carved this off from the JSON-settings refactor, and as far as I can tell #2176 stands alone against the existing
.psd1path. If #2134 lands later, both new cmdlets will need format-aware updates — but that's expected and not a blocker here. - vscode-powershell connection. Have you seen vscode-powershell#5385? Users keep getting bitten by the extension's effective defaults differing from
Invoke-ScriptAnalyzer's defaults, and the workaround they're given is "author aPSScriptAnalyzerSettings.psd1by hand," which is exactly the cliff this PR makes shorter.New-ScriptAnalyzerSettingsFile -BaseOnPreset <…>plusTest-ScriptAnalyzerSettingsFiletogether give them a discoverable starting point and a way to confirm it parses. Worth calling out in the docs/examples? - @bergmeister — would value your read on this one before merging, particularly on (1) maintenance posture and on whether the
RuleOptionInfoheuristic is something we want to lock in or constrain.
Me: Overally I support merging this. As a new feature I'm less inclined to carefully pick through the whole implementation and will defer to you, especially given the thorough tests, but want to be certain you'll be around to help maintain it if people start using it.
— Drafted by Copilot (Claude Opus 4.7)
Co-authored-by: Sean Wheeler <sean.wheeler@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class settings file discoverability and validation to PSScriptAnalyzer by introducing cmdlets to generate a complete default settings PSD1 (optionally based on a preset) and to validate a settings file’s rule names/options/types, while also exposing configurable rule option metadata via Get-ScriptAnalyzerRule.
Changes:
- Add
New-ScriptAnalyzerSettingsFileto generatePSScriptAnalyzerSettings.psd1(all rules/default options or preset-based). - Add
Test-ScriptAnalyzerSettingsFileto validate settings files and emitDiagnosticRecords (or$true/$falsewith-Quiet). - Extend
Get-ScriptAnalyzerRulewith anOptionsproperty and adjust custom rule path handling for empty arrays.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Engine/NewScriptAnalyzerSettingsFile.tests.ps1 | Adds Pester coverage for generating default/preset settings files, overwrite behavior, and -WhatIf. |
| Tests/Engine/TestScriptAnalyzerSettingsFile.tests.ps1 | Adds Pester coverage for validation scenarios (unknown rules/options, bad types/values, parse errors, -Quiet). |
| Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | Adds tests asserting Options presence/ordering/possible-values behavior. |
| Engine/Commands/NewScriptAnalyzerSettingsFileCommand.cs | Implements settings file generation cmdlet, including preset support and formatting. |
| Engine/Commands/TestScriptAnalyzerSettingsFileCommand.cs | Implements settings file validation cmdlet emitting diagnostics or boolean. |
| Engine/Commands/GetScriptAnalyzerRuleCommand.cs | Populates RuleInfo.Options for configurable rules. |
| Engine/Generic/RuleOptionInfo.cs | Introduces option metadata extraction (default values + inferred enum possible values). |
| Engine/Generic/RuleInfo.cs | Adds Options property and constructor overload to carry option metadata. |
| Engine/Helper.cs | Treats an empty resolved CustomRulePath list as null to align semantics with “no custom paths”. |
| Engine/Strings.resx | Adds new localized strings for preset/settings file creation & validation errors. |
| Engine/PSScriptAnalyzer.psm1 | Registers argument completer for New-ScriptAnalyzerSettingsFile -BaseOnPreset. |
| Engine/PSScriptAnalyzer.psd1 | Exports the new cmdlets from the module manifest. |
| docs/Cmdlets/New-ScriptAnalyzerSettingsFile.md | Adds documentation for the new settings file generation cmdlet. |
| docs/Cmdlets/Test-ScriptAnalyzerSettingsFile.md | Adds documentation for the new settings file validation cmdlet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@sdwheeler - Thanks for the docs review - all changes applied. @andyleejordan - I don't plan to go anywhere 🙂 - happy to pick up issues and PRs around these if they're merged. Happy to take a steer on the enum discovery. It works in all our current cases, but I appreciate there's not a guarantee that it would work in future cases. I really wanted to give some indication of what options there were for the non-obvious settings. I would love to see the vscode extension use these new cmdlets to allow people to add settings files easily to their projects with a code action. Also to allow some validation of an open settings file. I'm sure some clever cookie could work out how. |
PR Summary
Extracted and built upon the settings discoverability of my PR #2134 (omitting all of the JSON file format bits and settings refactoring).
New-ScriptAnalyzerSettingsFile, which generates complete settings files with everything set to it's default value.Test-ScriptAnalyzerSettingsFile, which validates a settings file (All rule names are valid, all settings valid etc).Get-ScriptAnalyzerRuleto include anOptionsproprety for configurable rules, including default value.ProcessCustomRulePathsEngine Helper to ignore an empty array ofCustomRulePaths.Motivation
It's not easy or convenient to find out what all the rules and rule settings available are. Your options are really to look at this repo, copy a preset/existing settings file, or look through all of the rules pages on MSLearn. Many people do not even know about settings files.
As a module developer, I want a way to discover what the available rules are and what options they have. I want an effortless way to add a settings file to my project that I can configure to my liking.
New-ScriptAnalyzerSettingsFileBy default this creates a ScriptAnalyzer settings file populated with all rule names and rule configuration (set to default values). The file has some comment help for each field.
The file is always named
PSScriptAnalyzerSettings.psd1so that automatic settings discovery inInvoke-ScriptAnalyzerpicks it up without any-Settingsparameter needed.Any rule configuration that is an enum - we attempt to list the possible values as a comment after the default value.
Example output of running
New-ScriptAnalyzerSettingsFilein this gistYou can optionally base the rule file on a preset. It takes the settings defined in the preset and normalises it all with the comments and all settings fields.
Example output of running
New-ScriptAnalyzerSettingsFile -BaseOnPreset CmdletDesignin this gist.Test-ScriptAnalyzerSettingsFileThis validates a settings file and tells you exactly what's wrong and where. It checks for:
IncludeRules,ExcludeRules, andRulesRulesthat aren't configurablebool,int,string,arrays)Kind = 'banana'when the valid values arespaceandtab)Each problem comes back as a
DiagnosticRecord, unless-Quietmode used - then just a boolean$true`$false` indicating whether the file is valid or not is returned.I debated for a while whether this should have just been a rule and after some chicken-or-egg thinking, decided it should be a separate cmdlet.
Get-ScriptAnalyzerRuleAdded an
Optionsproperty to the RuleInfo. I have not updated the format file, so it's not shown by default. I didn't know the implications of updating the format file.Running
Get-ScriptAnalyzerRule -Name PSUseConsistentIndentation | select -expand Optionsgets you:Successful test run in my fork.
PR Checklist
.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.